Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bugfix: initialize the network log #956

Merged
merged 1 commit into from
Mar 24, 2018
Merged

bugfix: initialize the network log #956

merged 1 commit into from
Mar 24, 2018

Conversation

rudyfly
Copy link
Collaborator

@rudyfly rudyfly commented Mar 24, 2018 •
edited by allencloud
Loading

Ⅰ. Describe what this PR did

revert commit: 8a70a80
libnetwork's logrus version is different from pouchd, so we need to
set libnetwork's logrus addintionly.

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

run pouchd and see network log format

Ⅴ. Special notes for reviews

also cc @faycheng

Signed-off-by: Rudy Zhang rudyflyzhang@gmail.com

@rudyfly
bugfix: initialize the network log
2bd3ffd 
revert commit: 8a70a80
libnetwork's logrus version is different from pouchd, so we need to
set libnetwork's logrus addintionly.

Signed-off-by: Rudy Zhang <rudyflyzhang@gmail.com>
@codecov-io
Copy link

Codecov Report

Merging #956 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #956   +/-   ##
=======================================
  Coverage   13.06%   13.06%           
=======================================
  Files         123      123           
  Lines        8235     8235           
=======================================
  Hits         1076     1076           
  Misses       7063     7063           
  Partials       96       96
Impacted Files Coverage Δ
daemon/mgr/network.go 3.95% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f1cee76...2bd3ffd. Read the comment docs.

@allencloud
Copy link
Collaborator

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Mar 24, 2018
@allencloud allencloud merged commit 979666b into AliyunContainerService:master Mar 24, 2018
@u2takey
Copy link
Contributor

u2takey commented Apr 25, 2018

cannot do make after this pr.
by the way, this is ugly way to fix "libnetwork's logrus version is different from pouchd"
best solution is fork logrus with specific version to such as "alibaba/libnetwork-logrus" then use alibaba/libnetwork-logrus when it is needed by libnetwork.

@allencloud
Copy link
Collaborator

best solution is fork logrus with specific version to such as "alibaba/libnetwork-logrus" then use alibaba/libnetwork-logrus when it is needed by libnetwork.

Thanks a lot for you advice. @u2takey
Please take a look at this to see whether this is reasonable. @rudyfly

@u2takey
Copy link
Contributor

u2takey commented Apr 25, 2018 •
edited
Loading

@rudyfly @allencloud may be not easy, because import path not match with actual git path.
Can we just upgrade libnetwork, which seems fixed this https://github.com/docker/libnetwork/pull/1856/files.
Or fix makefile so that users can build pouch smoothly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
areas/log areas/network kind/bug This is bug report for project LGTM one maintainer or community participant agrees to merge the pull reuqest. size/XS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rudyfly @codecov-io @allencloud @u2takey @pouchrobot